Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow defining schema format other than default #910

Merged
merged 17 commits into from
May 30, 2023

Conversation

GreenRover
Copy link
Collaborator

Please see issue #622

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving the suggestion here, as also voiced in today's spec 3.0 meeting.

Leave everything out that has to do with the new schema format that is now already known, and only focus on achieving the best structure possible, which enables us to add schemas with different schema formats in the components section.

This is to avoid clashing different discussions as it happened in today's meeting 🙂

@GreenRover
Copy link
Collaborator Author

I was thinking twice about "AsyncApi Schema object"
If i remove this again that would allow:

  • having avro schema for header
  • having avro schema for parameters

And the complexity will grow:

messageId: userSignup
name: UserSignup
title: User signup
summary: Action to sign a user up.
description: A longer description
contentType: application/json
tags:
  - name: user
  - name: signup
  - name: register
headers:
  schemaFormat: application/vnd.aai.asyncapi;version=3.0.0
  schema:
    type: object
    properties:
      correlationId:
  	    description: Correlation ID set by application
  	    type: string
      applicationInstanceId:
  	    description: Unique identifier for a given instance of the publishing application
  	    type: string
payload:
  schemaFormat: application/vnd.aai.asyncapi;version=2.2.0
  schema:
    type: object
    properties:
      schema:
        user:
          $ref: "#/components/asyncApiSchemas/userCreate"
        signup:
          $ref: "#/components/asyncApiSchemas/signup"

Do we really want that?

@smoya smoya mentioned this pull request Mar 24, 2023
@smoya smoya changed the title #622 allow defining schmea format other that default #622 allow defining schema format other than default Mar 25, 2023
@smoya
Copy link
Member

smoya commented Mar 27, 2023

Hi @GreenRover! Would you mind providing an update on how this is going? Are you still waiting for an answer on #910 (comment) or has been resolved after last meeting?

Thank you so much!

@GreenRover
Copy link
Collaborator Author

i still wait for response. needs to be discussed in next spec 3 meeting as it seams

@GreenRover
Copy link
Collaborator Author

how can we get any progress in this discussion?
@jonaslagoni may be a dedicated discussion?

@GreenRover
Copy link
Collaborator Author

Support for custom schema in header section was add.
Any comments from community?

@smoya
Copy link
Member

smoya commented Apr 11, 2023

Support for custom schema in header section was add. Any comments from community?

We should add a line somewhere mentioning there is no support for mixed schemas.

@GreenRover
Copy link
Collaborator Author

in my proposal this is not possible by design.
So no need for this line

@GreenRover
Copy link
Collaborator Author

GreenRover commented Apr 13, 2023

In today spec3.0 call https://youtu.be/hN6aE3Ebn08?t=1105

We came up with 3 options.
And a problem that will be solved later:

  • parameters support only basic types. And should not be specified with a full blown AsyncApi spec. A subtype is required.

Option 1a:

A separated component section.
Renaming asyncApiSchemas to schemas.

channels:
  userSignUp:
    address: 'users.{userId}'
    messages:
      userSignUp:
        $ref: '#/components/messages/userSignUp'
    parameters:
      userId:
        $ref: '#/components/parameters/userId'
components:
  messages:
    userSignUp:
      headers:
        $ref: '#components/schemas/MyHeaderSchema'
      payload:
        $ref: '#components/schemas/MyPayloadSchema'
  schemas:
    MyHeaderSchema:
      // schemaFormat is not set == default: application/vnd.aai.asyncapi;version=3.0.0
      schema:
        $ref: '#components/asyncApiSchemas/MyPayloadSchema'
    MyPayloadSchema:
      schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
      schema:
        $ref: '#components/asyncApiSchemas/MyPayloadSchema'
  asyncApiSchemas: // in spec < 3.0 this was named "schema"
    MyHeaderSchema:
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
    MyPayloadSchema:
      type: object
      properties:
        username:
          type: string
        email:
          type: string
  parameters:
    userId:
      description: The ID of the user.
      schema:
        type: string

Option 1b:

A separated component section.
Add new multiFormatSchemas component.

channels:
  userSignUp:
    address: 'users.{userId}'
    messages:
      userSignUp:
        $ref: '#/components/messages/userSignUp'
    parameters:
      userId:
        $ref: '#/components/parameters/userId'
components:
  messages:
    userSignUp:
      headers:
        $ref: '#components/multiFormatSchemas/MyHeaderSchema'
      payload:
        $ref: '#components/multiFormatSchemas/MyPayloadSchema'
    userSignIn: // inline insted of using `multiFormatSchemas`
      headers:
        schema:
          $ref: '#components/schema/MyHeaderSchema'
      payload:
        schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
        schema:
          $ref: '#components/schema/MyPayloadSchema'
  multiFormatSchemas:
    MyHeaderSchema:
      // schemaFormat is not set == default: application/vnd.aai.asyncapi;version=3.0.0
      schema:
        $ref: '#components/schemas/MyPayloadSchema'
    MyPayloadSchema:
      schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
      schema:
        $ref: '#components/schemas/MyPayloadSchema'
  schemas: // in spec < 3.0 this was named "schema"
    MyHeaderSchema:
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
    MyPayloadSchema:
      type: object
      properties:
        username:
          type: string
        email:
          type: string
  parameters:
    userId:
      description: The ID of the user.
      schema:
        type: string

Option 2a:

components.schemas get a new format.

schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
  • is optional. Default is async api schema in version of the spec.
  schema:
    $ref: 'https://schema.server.com/schemaXyz#/schema'
  • as components.schemas in spec < 3.0

OR

  $ref: 'https://schema.server.com/schemaXyz#/schema'
  • schema is optional. Default is async api schema in version of the spec.

Problem for the parser is, that $ref is only permitted to point to sub schemas with exact the same schemaFormat.

  • Mixing async api 2 and 3 is not permitted.
  • Mixing async and openapi is not permitted.
  • Mixing openapi and avro is not permitted.
channels:
  userSignUp:
    address: 'users.{userId}'
    messages:
      userSignUp:
        $ref: '#/components/messages/userSignUp'
    parameters:
      userId:
        $ref: '#/components/parameters/userId'
components:
  messages:
    userSignUp:
      headers:
        $ref: '#components/schemas/MyHeaderSchemaA'
      payload:
        $ref: '#components/schemas/MyPaloadAvroSchema'
    userSignIn:
      headers:
        $ref: '#components/schema/MyHeaderSchemaB'
      payload:
        $ref: '#components/schema/MyPaloadAsyncApiSchema'
  schemas: // can be of type "AsyncApi Schema object" or "Multi Format Schema Object"
    MyHeaderSchemaA:
      // schemaFormat is not set == default: application/vnd.aai.asyncapi;version=3.0.0
      schema:
        type: object
        properties:
          applicationInstanceId:
            description: Unique identifier for a given instance of the publishing application
            type: string
          
    MyHeaderSchemaB:
      // "schema" is missing so "AsyncApi Schema object" in version of document is assumed
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
          
  
    MyPaloadAvroSchema:
      schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
      schema: // refference to external schema server
        $ref: 'https://schema.server.com/schemaXyz#/schema'
    MyPaloadAsyncApiSchema:
      schema:
        type: object
        properties:
          username:
            type: string
          address:
            $ref: '#components/schemas/address/schema' 
    address:
      schema:
        type: object
        properties:
          town:
            type: string
          zip:
            type: string
          street:
            type: string
  parameters:
    userId:
      description: The ID of the user.
      schema:
        type: string

Option 2b:

components.schemas get a new format.

  schema:
    $ref: 'https://schema.server.com/schemaXyz#/schema'
  • as components.schemas in spec < 3.0

OR

  $ref: 'https://schema.server.com/schemaXyz#/schema'
  • schema is optional. Default is async api schema in version of the spec.

Problem for the parser is, that $ref is only permitted to point to sub schemas with exact the same schemaFormat.

  • Mixing async api 2 and 3 is not permitted.
  • Mixing async and openapi is not permitted.
  • Mixing openapi and avro is not permitted.
channels:
  userSignUp:
    address: 'users.{userId}'
    messages:
      userSignUp:
        $ref: '#/components/messages/userSignUp'
    parameters:
      userId:
        $ref: '#/components/parameters/userId'
components:
  messages:
    userSignUp:
      headers:
        $ref: '#components/schemas/MyHeaderSchema'
      payload:
        $ref: '#components/schemas/MyPaloadAvroSchema'
    userSignIn:
      headers:
        $ref: '#components/schema/MyHeaderSchema'
      payload:
        $ref: '#components/schema/MyPaloadAsyncApiSchema'
  schemas: // can be of type "AsyncApi Schema object" or "Multi Format Schema Object"
    MyHeaderSchemaA:
      // "schema" is missing so "AsyncApi Schema object" in version of document is assumed
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
          
  
    MyPaloadAvroSchema:
      schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
      schema: // refference to external schema server
        $ref: 'https://schema.server.com/schemaXyz#/schema'
    MyPaloadAsyncApiSchema:
      type: object
      properties:
        username:
          type: string
        address:
          $ref: '#components/schemas/address/schema' 
    address:
      schema:
        type: object
        properties:
          town:
            type: string
          zip:
            type: string
          street:
            type: string
  parameters:
    userId:
      description: The ID of the user.
      schema:
        type: string

Option 3:

components.schemas get a new sub level for each sub schema level.

Problem: references $ref: '#components/schemas/application~1vnd.aai.asyncapi;version=2.4.0/address' get really long and ugly. To mitigate the problem, there is a alias asyncapi for AsyncApi specs in same version as info.version of spec.

channels:
  userSignUp:
    address: 'users.{userId}'
    messages:
      userSignUp:
        $ref: '#/components/messages/userSignUp'
    parameters:
      userId:
        $ref: '#/components/parameters/userId'
components:
  messages:
    userSignUp:
      headers:
        $ref: '#components/schemas/asyncapi/MyHeaderSchema'
      payload:
        $ref: '#components/schemas/application~1vnd.apache.avro;version=1.9.0/MyPaloadAvroSchema'
    userSignIn:
      headers:
        $ref: '#components/schema/MyHeaderSchemaB'
      payload:
        $ref: '#components/schema/asyncapi/MyPaloadAsyncApiSchema'
    asyncApi2.4Sample:
      headers:
        $ref: '#components/schema/MyHeaderSchemaB'
      payload:
        $ref: '#components/schemas/application~1vnd.aai.asyncapi;version=2.4.0/MyPaloadAsyncApiSchema'
  schemas:
    asyncapi: // well defined alias for async api in same version as defined in `info.version`.
      MyHeaderSchema:
        type: object
        properties:
          applicationInstanceId:
            description: Unique identifier for a given instance of the publishing application
            type: string
      MyPaloadAsyncApiSchema:
        type: object
        properties:
          username:
            type: string
          address:
            $ref: '#components/schemas/asyncapi/address' 
          address:
            type: object
            properties:
              town:
                type: string
              zip:
                type: string
              street:
                type: string
    application/vnd.aai.asyncapi;version=2.4.0:
      MyPaloadAsyncApiSchema:
        properties:
          username:
            type: string
          address:
            $ref: '#components/schemas/application~1vnd.aai.asyncapi;version=2.4.0/address' 
          address:
            type: object
            properties:
              town:
                type: string
              zip:
                type: string
              street:
                type: string
  application/vnd.apache.avro;version=1.9.0:
    MyPaloadAvroSchema:
     $ref: 'https://schema.server.com/schemaXyz#/schema'
  
  application/vnd.google.proto;version=3: 
    MyProtoType: |
        syntax = "proto3";

        // only a single root type is permitted
        message MyProtoType {
          string query = 1;
          int32 page_number = 2;
          int32 result_per_page = 3;
        }

@smoya
Copy link
Member

smoya commented Apr 15, 2023

Option 2 seems the most reasonable to me. It seems the best in terms of DX and doesn't look like a hack to me.

@jonaslagoni
Copy link
Member

jonaslagoni commented Apr 18, 2023

I agree with @smoya here, option 2 seems like the better option.

Mixing async api 2 and 3 is not permitted.
Mixing async and openapi is not permitted.
Mixing openapi and avro is not permitted.

Maybe a better clarification is to say; mixing between schema formats is NOT permitted under any circumstance. 🤔

    // schemaFormat is not set == default: application/vnd.aai.asyncapi;version=3.0.0
      schema:
        type: object
        properties:
          applicationInstanceId:
            description: Unique identifier for a given instance of the publishing application
            type: string

Just to be sure, we are talking about letting components.schemas have a type of union between schema + schemaFormat (both required) or just AsyncAPI Schema Object, right?

Is there any reason to have schemaFormat optional?

@GreenRover
Copy link
Collaborator Author

Is there any reason to have schemaFormat optional?

To make it as easy as possible for AsyncApiSchema users

@jonaslagoni
Copy link
Member

@GreenRover but either a user are using AsyncAPI Schema object, or they are not.

If they are using the AsyncAPI Schema object it makes no sense to indent it within a .schema.

If they are not using the AsyncAPI Schema object they would specify both schema and schemaFormat, right? 🤔

I just see the optional schemaFormat variant as providing more confusion 🤔

@jonaslagoni
Copy link
Member

jonaslagoni commented Apr 18, 2023

I.e. this is the optimal document in my eyes:

  schemas: // can be of type "AsyncApi Schema object" or "Multi Format Schema Object"
    MyHeaderSchemaA:
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
    MyHeaderSchemaB:
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string
    MyPaloadAvroSchema:
      schemaFormat: 'application/vnd.apache.avro+yaml;version=1.9.0'
      schema: // refference to external schema server
        $ref: 'https://schema.server.com/schemaXyz#/schema'
    MyPaloadAsyncApiSchema:
        properties:
          username:
            type: string
          address:
            $ref: '#components/schemas/address' 
    address:
        type: object
        properties:
          town:
            type: string
          zip:
            type: string
          street:
            type: string

@GreenRover
Copy link
Collaborator Author

@jonaslagoni you are right. And won the version "2b"
I updated my comment (5 days ago)
@derberg Please choose between 2a and 2b

@fmvilas
Copy link
Member

fmvilas commented Apr 18, 2023

I'm not fully sure what's the difference between 2a and 2b, would you mind putting an example? With that said, I think I'm more inclined to 2a but just want to make sure I fully understand it. Thanks for taking the time to lead this discussion, Heiko! 🙌

@GreenRover
Copy link
Collaborator Author

The examples are here:
#910 (comment)

The difference:
2a allows:

  schemas: // can be of type "AsyncApi Schema object" or "Multi Format Schema Object"
    MyHeaderSchemaA:
      // schemaFormat is not set == default: application/vnd.aai.asyncapi;version=3.0.0
      schema:
        type: object
        properties:
          applicationInstanceId:
            description: Unique identifier for a given instance of the publishing application
            type: string
          
    MyHeaderSchemaB:
      // "schema" is missing so "AsyncApi Schema object" in version of document is assumed
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string

and 2b only:

  schemas: // can be of type "AsyncApi Schema object" or "Multi Format Schema Object"
    MyHeaderSchemaB:
      // "schema" is missing so "AsyncApi Schema object" in version of document is assumed
      type: object
      properties:
        applicationInstanceId:
          description: Unique identifier for a given instance of the publishing application
          type: string

So 2b give you one way less to solace the same problem.

@derberg
Copy link
Member

derberg commented Apr 18, 2023

2a 🚀

so basically example from Jonas -> #910 (comment)
(@jonaslagoni MyPaloadAsyncApiSchema is missing type: object right?)

Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not sure I understand what "In case that a Schema Object the schemaFormat will be assumed as..." means.

Can we rephrase the three occurrences of this to be clearer, please?

Am I right in guessing that it's saying "If this is a Schema Object, then the schemaFormat will be assumed to be..." ?

@GreenRover
Copy link
Collaborator Author

Sorry, I'm not sure I understand what "In case that a Schema Object the schemaFormat will be assumed as..." means.

@dalelane This was phrase was modified in mean while. Better now?

@dalelane
Copy link
Collaborator

Sorry, I'm not sure I understand what "In case that a Schema Object the schemaFormat will be assumed as..." means.

@dalelane This was phrase was modified in mean while. Better now?

Where was it changed? I don't see any changes to the pull request.

image

@GreenRover
Copy link
Collaborator Author

@dalelane Ok i found this sentence and changed it now as suggested

Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was clearer - thank you

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@fmvilas fmvilas changed the title #622 allow defining schema format other than default feat: allow defining schema format other than default May 29, 2023
@fmvilas
Copy link
Member

fmvilas commented May 29, 2023

Changed the PR title to meet the requirements.

@GreenRover
Copy link
Collaborator Author

/rtm

@asyncapi-bot
Copy link
Contributor

Hello, @GreenRover! 👋🏼
This PR is not up to date with the base branch and can't be merged.
Please update your branch manually with the latest version of the base branch.
PRO-TIP: Add a comment to your PR with the text: /au or /autoupdate and our bot will take care of updating the branch in the future. The only requirement for this to work is to enable Allow edits from maintainers option in your PR.
Thanks 😄

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg
Copy link
Member

derberg commented May 30, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit 3c61f49 into asyncapi:next-major-spec May 30, 2023
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.0.0-next-major-spec.12 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants